Skip to content

feat(nix): install packaged shell completions - #947

Open
betaboon wants to merge 5 commits into
Fission-AI:mainfrom
betaboon:feat-nix-flake-completion
Open

feat(nix): install packaged shell completions#947
betaboon wants to merge 5 commits into
Fission-AI:mainfrom
betaboon:feat-nix-flake-completion

Conversation

@betaboon

@betaboon betaboon commented Apr 10, 2026

Copy link
Copy Markdown

Status

Implementation LGTM after independent review. The Nix packaging policy is approved: bundle completion files, let the shell control activation, and leave user startup files untouched. All required CI checks and Security passed for implementation commit 3cd948038df2157c2cb09fe2980345666f471f39; the follow-up only clarifies documentation. Not merged.

Motivation

Nix users should receive completion files with the package, without running a separate installer or modifying shell startup files. Keeping the files in the package also keeps them aligned with the installed CLI version.

What it does

  • Installs Bash, Fish, and Zsh completions through Nixpkgs' installShellFiles hook.
  • Generates files only when the build platform can execute the host platform.
  • Runs generation sequentially so a nonzero exit fails the build, including after partial output.
  • Checks in CI that all three packaged files match fresh CLI output.
  • Documents activation, installer ownership, and tip behavior for Nix completions.

Proof it works

  • GitHub CI: Linux, macOS, and Windows tests, lint/type checking, Nix build and packaged-completion verification, and the required aggregate check all pass.
  • GitHub Security: dependency review, audits, and website lockfile validation pass.
  • Real Nix build on aarch64-linux; packaged CLI reports 1.11.0.
  • All three installed files are nonempty, byte-identical to fresh output, and pass their shell's syntax check.
  • nix flake check --no-build --all-systems evaluates all four supported systems.
  • Exact-hook failure injection with Nixpkgs' installer: a generator that prints partial output then exits 17 is incorrectly accepted by the original hook (exit 0); the hardened hook exits 17 before installation.
  • Type checking, lint, and 333 focused completion tests pass.
  • Full suite in an isolated Linux environment with Node.js 22.21.1: 4,229 tests pass across 145 files.

Notes

  • Approved policy: Nix packages include completion files as a documented exception to explicit CLI installation. Shell configuration controls activation and may load them automatically. OpenSpec does not modify startup files when installing the Nix package.
  • The CLI reference now distinguishes package-managed completions from user-local copies. CLI install/uninstall and installed-file detection manage the latter, not the immutable Nix package. The installation guide links to that explanation. OPENSPEC_NO_COMPLETIONS=1 suppresses the tip; it does not disable active completions. No core-code expansion is proposed to manage the Nix store.
  • Reconciled the branch with current main; retained its Node.js and dependency versions. No application code or dependency changes.
  • Does not edit .bashrc, .zshrc, or Fish configuration. Users still need their shell's completion subsystem enabled.
  • Telemetry is disabled only during generation and verification, not in the installed CLI.
  • Host-wide tests exposed existing environment leaks through Oh My Zsh and global MiniMax skills. Those user settings were left untouched; all affected cases pass in the clean Linux full-suite run.

@betaboon
betaboon requested a review from TabishB as a code owner April 10, 2026 17:47

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c4b9994-406f-449a-ac02-4c7948903a3c

📥 Commits

Reviewing files that changed from the base of the PR and between a0ddb60 and 3cd9480.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • flake.nix
🚧 Files skipped from review as they are similar to previous changes (1)
  • flake.nix

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The Nix derivation now packages Bash, Fish, and Zsh completions when the build platform can execute the host platform. CI generates each completion with telemetry disabled and compares it with the packaged file.

Changes

Nix shell completion packaging

Layer / File(s) Summary
Completion packaging and validation
flake.nix, .github/workflows/ci.yml
The derivation adds installShellFiles and conditionally generates completions with OPENSPEC_TELEMETRY=0. CI checks that Bash, Fish, and Zsh completions are non-empty and match the packaged files.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 3cd94

This change packages Bash, Fish, and Zsh completions and validates them in CI; it is localized and introduces no actionable merge-blocking risk beyond normal checks and review.

Suggested reviewers: tabishb

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: installing packaged shell completions in the Nix package.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@betaboon
betaboon force-pushed the feat-nix-flake-completion branch from d98d602 to 4cad923 Compare April 17, 2026 08:39
@betaboon

Copy link
Copy Markdown
Author

@TabishB is there anything missing here? :)

@betaboon
betaboon force-pushed the feat-nix-flake-completion branch from 669f581 to 253838a Compare April 22, 2026 06:36
@TabishB

TabishB commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

@betaboon we've made completion installation opt-in recently. would be good to make it standard across the board.

Mainly because of this issue: #948
ref PR: #949

@betaboon

Copy link
Copy Markdown
Author

it's rather unusual in nixpkgs to make the installation of completions optional.
The upstream package-definition for OpenSpec also just installs the completions.

The problem described in #948 is pretty much ruled out in nix/nixos.

some additional detail:
the postinstall is not used here, just the openspec completion generate command and the output is written to $out/share/bash-completions etc.

@clay-good
clay-good requested a review from a team as a code owner August 27, 2026 21:29
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@clay-good clay-good changed the title feat(nix): install completions feat(nix): install packaged shell completions Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants